Skip to content

Feat/pim/m20 kronknav - #3776

Merged
leshy merged 17 commits into
mainfrom
feat/pim/m20-kronknav
Sep 4, 2026
Merged

Feat/pim/m20 kronknav#3776
leshy merged 17 commits into
mainfrom
feat/pim/m20-kronknav

Conversation

@Nabla7

@Nabla7 Nabla7 commented Aug 31, 2026

Copy link
Copy Markdown
Collaborator
  • Adds the M20 integration machinery: native ROS/DDS command bridge, robot lifecycle/control, direct lidar and IMU ingestion into Point-LIO, KronkNav mapping/planning/control, and front/rear camera streams.

  • Extends NativeModuleConfig with optional Linux CPU affinity so Point-LIO, ray tracing, and MLS can be assigned to appropriate RK3588 cores, this is necessary as otherwise performance becomes severely degraded.

Some notes:

  • This runs fully on the RK3588, further tuning will be required as the mapper is always running behind and this gets worse as the global map grows, the bottleneck is RayTracingVoxelMap on this device.

  • Planning on flat terrain works consistently, planning over stairs is hit or miss. Naturally since the map is always behind dynamic obstacle avoidance does not work.

How to run:

On the M20:
Configure permissions:
./dimos/robot/deeprobotics/m20/deploy/setup.sh

Start blueprint:
dimos --transport lcm --rerun-host 0.0.0.0 run deeprobotics-m20-kronknav-control

Attach the control shell:
dimos --transport lcm shell
app.M20Connection.standup()
app.M20Connection.liedown()

On laptop:
dimos-viewer --connect rerun+http://10.21.31.104:9877/proxy
--ws-url ws://10.21.31.104:3030/ws

m20forgithub.mov

@codecov

codecov Bot commented Aug 31, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 45.26627% with 185 lines in your changes missing coverage. Please review.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
dimos/robot/deeprobotics/m20/connection.py 36.86% 137 Missing ⚠️
dimos/robot/deeprobotics/m20/camera.py 50.66% 37 Missing ⚠️
.../robot/deeprobotics/m20/blueprints/m20_kronknav.py 71.05% 11 Missing ⚠️
@@            Coverage Diff             @@
##             main    #3776      +/-   ##
==========================================
- Coverage   78.08%   77.99%   -0.10%     
==========================================
  Files        1380     1384       +4     
  Lines      130324   130662     +338     
  Branches    11310    11745     +435     
==========================================
+ Hits       101761   101907     +146     
- Misses      25320    25512     +192     
  Partials     3243     3243              
Flag Coverage Δ
OS-ubuntu-24.04-arm 74.31% <45.26%> (-0.08%) ⬇️
OS-ubuntu-latest 74.82% <45.26%> (-0.08%) ⬇️
Py-3.10 74.81% <45.26%> (-0.08%) ⬇️
Py-3.11 74.81% <45.26%> (-0.09%) ⬇️
Py-3.12 74.81% <45.26%> (-0.08%) ⬇️
Py-3.13 74.81% <45.26%> (-0.08%) ⬇️
Py-3.14 74.82% <45.26%> (-0.08%) ⬇️
Py-3.14t 74.82% <45.26%> (-0.08%) ⬇️
SelfHosted-Large 30.75% <ø> (-0.01%) ⬇️
SelfHosted-Linux 35.78% <ø> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
dimos/robot/all_blueprints.py 100.00% <ø> (ø)
dimos/robot/deeprobotics/m20/constants.py 100.00% <100.00%> (ø)
.../robot/deeprobotics/m20/blueprints/m20_kronknav.py 71.05% <71.05%> (ø)
dimos/robot/deeprobotics/m20/camera.py 50.66% <50.66%> (ø)
dimos/robot/deeprobotics/m20/connection.py 36.86% <36.86%> (ø)

... and 4 files with indirect coverage changes

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@Nabla7
Nabla7 marked this pull request as ready for review August 31, 2026 09:46
@github-actions github-actions Bot added the ready-to-merge Required CI checks have passed on this PR label Aug 31, 2026
@greptile-apps

greptile-apps Bot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This change adds M20 ROS/DDS control, Point-LIO localization, navigation components, camera relays, deployment support, and CPU-affinity handling for native children.

A malformed lidar PointCloud2 message can pass Point-LIO field validation when a coordinate field begins near the end of a point record. The subsequent native coordinate read extends past the record boundary, producing undefined behavior and potentially terminating localization.

Confidence Score: 3/5

Not safe to merge until Point-LIO rejects coordinate fields that extend past a point record.

The affected validation and native read path were exercised with a valid control case and a malformed boundary-crossing coordinate field. AddressSanitizer reported the predicted out-of-bounds read for the malformed input.

Files Needing Attention: dimos/robot/deeprobotics/m20/pointlio/cpp/main.cpp

Security Review

Point-LIO accepts malformed FLOAT32 x, y, or z field offsets that begin within point_step but do not leave room for the full four-byte value. A focused AddressSanitizer reproduction confirmed that the accepted message causes an out-of-bounds native read. Rejecting coordinate fields whose typed width exceeds point_step prevents corrupted or untrusted lidar traffic from reaching this unsafe read.

T-Rex T-Rex Logs

What T-Rex did

  • Ran the focused parser reproduction script to reproduce the P1 finding and reviewed the related review comment.
  • Opened and inspected the focused parser reproduction source and its accompanying run logs to confirm the repro setup and expectations.
  • Validated the general contract by confirming that x_offset=0 is accepted and all reads complete with exit 0, and that x_offset=31 triggers an AddressSanitizer-reported over-read.
  • Checked the toolchain readiness using the ASan harness build log, valid point execution, stack trace, and native M20 environment result.

View all artifacts

T-Rex Ran code and verified through T-Rex

Comments Outside Diff (1)

  1. General comment

    P1 Coordinate field widths are not validated before native reads

    • Bug
      • validate_m20_cloud accepts a FLOAT32 x, y, or z field when its offset is merely below point_step. A one-point cloud with point_step=32, exactly 32 bytes of data, and x.offset=31 passes validation. The subsequent read_unaligned<float> copies four bytes from offset 31, which AddressSanitizer confirmed reads beyond the point/data allocation.
    • Cause
      • The bounds check at lines 157-160 covers timestamp, ring, and intensity only; x/y/z are omitted despite being read as four-byte floats at lines 544-546.
    • Fix
      • Extend the point-step validation to require offsets.x + sizeof(float), offsets.y + sizeof(float), and offsets.z + sizeof(float) to be no greater than cloud.point_step (ideally through one common typed-field bounds helper).

    T-Rex Ran code and verified through T-Rex

Reviews (1): Last reviewed commit: "chore(robot): automate M20 provisioning" | Re-trigger Greptile

Comment thread dimos/robot/deeprobotics/m20/pointlio/cpp/main.cpp Outdated
Comment thread dimos/robot/deeprobotics/m20/pointlio/module.py Outdated
Comment thread dimos/robot/deeprobotics/m20/pointlio/module.py Outdated
Comment thread dimos/robot/deeprobotics/m20/blueprints/m20_kronknav.py Outdated
Comment thread dimos/robot/deeprobotics/m20/pointlio/cpp/main.cpp Outdated
@leshy

leshy commented Sep 1, 2026

Copy link
Copy Markdown
Member

this robot already runs vendor configured pointlio on the nav machine so we shouldn't run another pointlio. there is a zenoh forwarder service that uses DRDDS already written in that PR

https://github.com/dimensionalOS/dimos/pull/2885/changes#diff-f152e876f1992a4ff070fba547f6ca05dadf7c65641dae75999aa7852791117b

@leshy

leshy commented Sep 1, 2026

Copy link
Copy Markdown
Member

video transforms seem to be missing from robot, camera feeds should be visible in 3d

Comment thread dimos/robot/deeprobotics/m20/deploy/dimos-m20-multicast-relay-supervisor Outdated
@leshy

leshy commented Sep 1, 2026

Copy link
Copy Markdown
Member

we don't have to run rerun on the robot with limited resources, you should connect via zenoh to the robot, can give info on this

@Nabla7

Nabla7 commented Sep 1, 2026

Copy link
Copy Markdown
Collaborator Author

we don't have to run rerun on the robot with limited resources, you should connect via zenoh to the robot, can give info on this

If you can point me to where this pattern is established already would be great.

@leshy

leshy commented Sep 1, 2026

Copy link
Copy Markdown
Member

we don't have to run rerun on the robot with limited resources, you should connect via zenoh to the robot, can give info on this

If you can point me to where this pattern is established already would be great.

https://github.com/dimensionalOS/dimos/pull/3572/changes#diff-8e0ae3cca6583f09a1fea072191c8e479fdf0120cb8b771f476b7b248b251f99R378

dimos --robot-ips "go2" run go2-zenoh-motion-local

@github-actions github-actions Bot removed the ready-to-merge Required CI checks have passed on this PR label Sep 1, 2026
@leshy
leshy enabled auto-merge September 1, 2026 20:05
leshy
leshy previously approved these changes Sep 1, 2026
Comment thread dimos/robot/deeprobotics/m20/connection.py Outdated
leshy
leshy previously approved these changes Sep 2, 2026

@leshy leshy left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm ok to merge, better design proposed with connection.py removed, replaced by onboard drdds bridge having connection interface

Comment thread dimos/robot/deeprobotics/m20/connection.py
@github-actions github-actions Bot added the ready-to-merge Required CI checks have passed on this PR label Sep 4, 2026
@leshy
leshy added this pull request to the merge queue Sep 4, 2026
Merged via the queue into main with commit f83b712 Sep 4, 2026
30 of 32 checks passed
@leshy
leshy deleted the feat/pim/m20-kronknav branch September 4, 2026 14:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-to-merge Required CI checks have passed on this PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants